-
Notifications
You must be signed in to change notification settings - Fork 20.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ethapi: Add tests for SetCodeTx type to check Call functionality #30947
base: master
Are you sure you want to change the base?
Conversation
@@ -1165,6 +1203,7 @@ func TestCall(t *testing.T) { | |||
t.Errorf("test %s, result mismatch, have\n%v\n, want\n%v\n", tc.name, result.String(), tc.want) | |||
} | |||
} | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pls remove
@@ -870,14 +870,15 @@ func TestCall(t *testing.T) { | |||
|
|||
// Initialize test accounts | |||
var ( | |||
accounts = newAccounts(3) | |||
accounts = newAccounts(7) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do you need 7 accounts here? afaict you only use 5
dad = common.HexToAddress("0x0000000000000000000000000000000000000dad") | ||
genesis = &core.Genesis{ | ||
Config: params.MergedTestChainConfig, | ||
Alloc: types.GenesisAlloc{ | ||
accounts[0].addr: {Balance: big.NewInt(params.Ether)}, | ||
accounts[1].addr: {Balance: big.NewInt(params.Ether)}, | ||
accounts[2].addr: {Balance: big.NewInt(params.Ether)}, | ||
accounts[3].addr: {Balance: big.NewInt(params.Ether) ,Code: append(types.DelegationPrefix, accounts[4].addr.Bytes()...)}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The file needs to be goimports -ed otherwise the linter will fail
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
Right, as far as I can tell these cases test that a delegated account is still able to transfer value and call contracts as before. Fair. Tho what I had in mind was testing the new functionality a bit. One of them being: doing a call where AuthorizationsList is non-empty and it tries to delegate an account. Another interesting one is using a 3rd account to call a delegated account. |
Okay will surely add that ! |
resolves #30920
I added tests similar that were pushed for eth_estimate gas what more tests can i add